feat(sandbox): add suspend and resume operations - #2653

Reviewed the full diff (excluding regenerated sdk/go/proto). The gateway state machine here is the strongest part of the change — durable intent before mutation, cancellation-safe workers, ambiguous-outcome reconciliation, and gate unification are all handled deliberately and backed by real tests. Most of my feedback is on the Kubernetes driver, where the suspend confirmation path has a timing problem that will surface on the happy path.

What works well

• Persist intent before mutating compute. Suspending/Resuming is CAS-written before the driver call, so a crash mid-transition is recoverable. recover_persisted_lifecycle_transitions (compute/mod.rs:2108) retries those rows at startup ahead of startup_resume, and sandbox_phase_should_be_running correctly excludes Suspended/Suspending so startup recovery can't wake a suspended sandbox.
• Detached workers so request cancellation can't strand a row. Once the durable transition commits, the driver call moves into an owned task holding the lifecycle gate and the RPC awaits the join handle. Mirrors the delete path, and request_cancellation_does_not_cancel_{suspend,resume}_worker cover both directions.
• DeleteGateRegistry → LifecycleGateRegistry. Suspend, resume, and delete now serialize against each other rather than only against themselves, with the gate → global-guard lock order still enforced by the guard token.
• Ambiguous-outcome reconciliation. recover_failed_lifecycle (compute/mod.rs:1265) re-queries the driver outside the global lock and retains the transition when backend state can't be resolved, rather than asserting a running/stopped state it can't verify.
• Resume requires a fresh supervisor session, and the apply_driver_snapshot clamp keeps a stale snapshot from promoting a suspended row.
• VM driver ordering is careful: the marker is written before process handles are detached and cleared only after all restore preflight checks pass, so a failed resume stays durably suspended (failed_resume_preserves_suspended_state).

I also confirmed against upstream agent-sandbox that the Kubernetes retention claim in the driver README is accurate: reconcilePVCs runs only inside reconcileChildResources, which the suspend path bypasses, so volumeClaimTemplates are left intact.

1. Kubernetes suspend will time out on the happy path and roll the row back to Ready

driver.rs:932-955

The API surface the driver targets is correct — spec.operatingMode (Running/Suspended) and the Suspended condition are both real published v1beta1 API. The problem is what the condition means. From upstream computeSuspendedCondition:

if pod == nil {
    suspended.Status = metav1.ConditionTrue
    suspended.Reason = sandboxv1beta1.SandboxReasonSuspendedPodTerminated
    ...
}
// pod still present:
suspended.Reason = sandboxv1beta1.SandboxReasonSuspendedPodTerminating
suspended.Message = "Pod is terminating. Sandbox is suspending"

Suspended=True requires the pod object to be fully gone from the API, not merely terminating. So the wait must cover the controller's reconcile, the pod's full grace period, kubelet teardown, and a re-reconcile — against a budget of KUBE_API_TIMEOUT = 30s. Nothing in this crate sets terminationGracePeriodSeconds on the sandbox pod (git grep -in grace crates/openshell-driver-kubernetes/ is empty), so it's the Kubernetes default of 30s. That leaves zero headroom: any sandbox whose supervisor or agent process doesn't exit promptly on SIGTERM blows the budget before the pod can possibly be gone.

The rollback is what makes this user-visible. On timeout, recover_failed_lifecycle(expected_stopped=true) re-queries the driver and sees Suspended=False/SuspendedPodTerminating plus Ready=False with a non-terminal reason. derive_phase returns Provisioning, and driver_snapshot_confirms_stopped is false (it only matches containerexited/containerstopped), so observed_stopped=false ≠ expected_stopped=true and restore_lifecycle_snapshot writes the row back to Ready. Net effect:

• openshell sandbox suspend fails with a timeout even though suspension is proceeding correctly.
• The phase flaps Suspending → Ready → Suspended (the watcher does eventually correct it, since old_phase = Ready falls through the clamp to _ => phase).
• cleanup_suspended_sandbox_sessions never runs on the watcher path, so stored SshSession records linger until the next explicit suspend or a gateway restart. The supervisor session dies with the pod, so that part is fine.

Two suggestions: derive the suspend wait budget from the pod's grace period instead of reusing the API-call timeout, and treat SuspendedPodTerminating as "in progress" in recover_failed_lifecycle so a progressing suspend retains Suspending rather than rolling back.

2. Mechanical issues in the same poll loop

driver.rs:939-948

• The api.get(&kube_name) inside the loop is the only kube call in this file not wrapped in tokio::time::timeout(KUBE_API_TIMEOUT, …), and the deadline is checked after the call returns. A hung API server hangs suspend indefinitely.
• Fixed 250 ms polling means up to ~120 GETs per suspend against the API server. Worth backing off, or watching instead.

3. status.replicas == 0 fallback is dead code

driver.rs:3222

Upstream v1alpha1 declares Replicas int32 with json:"replicas,omitempty" — non-pointer int with omitempty, so zero is dropped at marshal time and status.get("replicas") == Some(0) can never match on a suspended sandbox. Separately, current upstream main has no v1alpha1 suspension logic at all: both computeSuspendedCondition and the pod-delete path key off spec.OperatingMode only, with no spec.Replicas == 0 check.

So it's worth confirming what the v1alpha1 path actually does on a real legacy install. If an older controller honors spec.replicas but doesn't publish the condition, suspend times out and rolls back to Ready while the pod is genuinely gone — silent divergence. If it ignores spec.replicas entirely, the patch is a no-op and the rollback is correct. Either way the fallback branch as written can't help, so I'd drop it and require the condition on both versions.

4. No driver capability negotiation for suspend/resume

proto/compute_driver.proto:47

ResumeSandbox is a required RPC and GetCapabilitiesResponse (:58) carries no feature flags. Against an out-of-tree driver that hasn't implemented it, the gateway durably writes Suspending and then fails with UNIMPLEMENTED. Recovery does roll back correctly, but a capability bit would let handle_suspend_sandbox reject with FAILED_PRECONDITION before touching durable state. This is also a compat break for external driver implementations and should be called out in release notes.

5. TUI not updated

crates/openshell-tui/src/lib.rs:2684

phase_label still handles only Provisioning/Ready/Error/Deleting, so suspended sandboxes render as "Unknown" in the dashboard. The CLI, both SDKs, docs, and agent skills were all updated — this is the one surface that was missed.

6. The Suspended clamp is unconditional

compute/mod.rs:3429

SandboxPhase::Suspended => SandboxPhase::Suspended,

Once a row is Suspended, no driver signal can move it out — including Error and Deleting. stopped_container_snapshot_cannot_error_suspended_sandbox shows this is intentional, and the absent-resource case is covered by the new ComputeResourceMissing branch. But an out-of-band failure that isn't absence (PVC lost, an external kubectl delete setting a deletionTimestamp) will keep reporting Suspended. Is that the intent, or should Deleting/Error pass through the clamp?

7. Stuck Resuming has no terminal state

If compute starts but the supervisor never reconnects, the row stays Resuming indefinitely. suspend rejects it (requires Ready|Suspending), nothing times it out to Error, and the CLI just expires at 300s — retry-resume or delete are the only exits. Consider a Resuming deadline, or allowing suspend from Resuming.

8. Phase and conditions can contradict

compute/mod.rs:3443

The clamp rewrites status.phase but leaves the conditions ComposedPhase::apply_readiness_conditions already wrote, so a stale running snapshot can leave phase = Suspended alongside Ready: True in openshell sandbox get output. Narrow window — it's exactly the case stale_ready_snapshot_cannot_wake_suspended_sandbox covers, and that test asserts only the phase.

9. Podman error remapping is broader than the feature

crates/openshell-driver-podman/src/driver.rs:39

PodmanApiError::NotFound → ComputeDriverError::NotFound changes every unexpected 404 in the Podman driver from INTERNAL to NOT_FOUND, not just the new stop/resume path — and the gateway interprets NOT_FOUND from delete_sandbox as "already gone." Likely an improvement, but it's a behavior change outside the issue's scope.

10. Go SDK SandboxInterface gains three methods

Suspend, Resume, and WaitSuspended are additions to a public interface, which breaks any external implementer or hand-written mock. Fine to do — it just belongs in release notes.

Minor

• OPENSHELL_LIFECYCLE_TIMEOUT (crates/openshell-cli/src/run.rs:2493) is undocumented in cli-reference.md and docs/.
• The PR description says suspend retains /workspace; the actual mount is /sandbox (WORKSPACE_MOUNT_PATH), which is what the E2E sentinel correctly uses.
• Docs don't state that how much survives differs by driver. Docker and Podman keep the whole container writable layer and the VM keeps the full overlay, but on Kubernetes the pod is recreated, so only /sandbox (the PVC) survives — installed packages and edits to /etc, /home, /tmp are lost. docs/sandboxes/manage-sandboxes.mdx currently reads as if resume is transparent; worth a sentence there since it's a user-visible expectation.
• recover_persisted_lifecycle_transitions (compute/mod.rs:2111) lists at most 1000 sandboxes with no pagination, silently skipping lifecycle recovery beyond that. Same limitation as the existing TODO in this file, but new code inherits it.

Items 1–3 are what I'd want resolved before merge, since the Kubernetes path is the least verified — the Testing section lists Docker E2E only — and item 1 makes the happy path report failure. 4–7 are design questions worth answering; the rest is cleanup.